Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: prevent panic on scrolling beyond buffer limits; #10671

Closed
wants to merge 1 commit into from
Closed

Fix: prevent panic on scrolling beyond buffer limits; #10671

wants to merge 1 commit into from

Conversation

5-pebbles
Copy link

Currently if you scroll to the bottom of the buffer, and then scroll down a popup menu helix will panic.

Error:

thread 'main' panicked at 'Trying to access position outside the buffer: x=17, y=33, area=Rect { x: 0, y: 0, width: 124, height: 33 }', helix-tui/src/buffer.rs:235:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Full Backtrace 
$ ~/apps/rust/helix/target/debug/hx
thread 'main' panicked at 'Trying to access position outside the buffer: x=19, y=33, area=Rect { x: 0, y: 0, width: 124, height: 33 }', helix-tui/src/buffer.rs:235:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/panicking.rs:67:14
   2: helix_tui::buffer::Buffer::index_of
             at /home/ghostbird/apps/rust/helix/helix-tui/src/buffer.rs:235:9
   3: <helix_tui::buffer::Buffer as core::ops::index::IndexMut<(u16,u16)>>::index_mut
             at /home/ghostbird/apps/rust/helix/helix-tui/src/buffer.rs:658:17
   4: <helix_term::ui::menu::Menu<T> as helix_term::compositor::Component>::render
             at /home/ghostbird/apps/rust/helix/helix-term/src/ui/menu.rs:400:33
   5: <helix_term::ui::popup::Popup<T> as helix_term::compositor::Component>::render
             at /home/ghostbird/apps/rust/helix/helix-term/src/ui/popup.rs:319:9
   6: <helix_term::ui::completion::Completion as helix_term::compositor::Component>::render
             at /home/ghostbird/apps/rust/helix/helix-term/src/ui/completion.rs:444:9
   7: <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::render
             at /home/ghostbird/apps/rust/helix/helix-term/src/ui/editor.rs:1563:13
   8: helix_term::compositor::Compositor::render
             at /home/ghostbird/apps/rust/helix/helix-term/src/compositor.rs:178:13
   9: helix_term::application::Application::render::{{closure}}
             at /home/ghostbird/apps/rust/helix/helix-term/src/application.rs:278:9
  10: helix_term::application::Application::handle_terminal_events::{{closure}}
             at /home/ghostbird/apps/rust/helix/helix-term/src/application.rs:651:26
  11: helix_term::application::Application::event_loop_until_idle::{{closure}}
             at /home/ghostbird/apps/rust/helix/helix-term/src/application.rs:320:55
  12: helix_term::application::Application::event_loop::{{closure}}
             at /home/ghostbird/apps/rust/helix/helix-term/src/application.rs:294:57
  13: helix_term::application::Application::run::{{closure}}
             at /home/ghostbird/apps/rust/helix/helix-term/src/application.rs:1194:38
  14: hx::main_impl::{{closure}}
             at /home/ghostbird/apps/rust/helix/helix-term/src/main.rs:161:53
  15: tokio::runtime::park::CachedParkThread::block_on::{{closure}}
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/park.rs:281:63
  16: tokio::runtime::coop::with_budget
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/coop.rs:107:5
  17: tokio::runtime::coop::budget
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/coop.rs:73:5
  18: tokio::runtime::park::CachedParkThread::block_on
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/park.rs:281:31
  19: tokio::runtime::context::blocking::BlockingRegionGuard::block_on
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/context/blocking.rs:66:9
  20: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::{{closure}}
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/scheduler/multi_thread/mod.rs:87:13
  21: tokio::runtime::context::runtime::enter_runtime
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/context/runtime.rs:65:16
  22: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/scheduler/multi_thread/mod.rs:86:9
  23: tokio::runtime::runtime::Runtime::block_on
             at /home/ghostbird/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.37.0/src/runtime/runtime.rs:351:45
  24: hx::main_impl
             at /home/ghostbird/apps/rust/helix/helix-term/src/main.rs:163:5
  25: hx::main
             at /home/ghostbird/apps/rust/helix/helix-term/src/main.rs:37:21
  26: core::ops::function::FnOnce::call_once
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

It's caused by the following code in helix-term/src/ui/menu.rs:

let render_borders = cx.editor.menu_border();

// this highlights the selected edges if there are no borders
if !render_borders {
    if let Some(cursor) = self.cursor {

        let offset_from_top = cursor - scroll;

        // this panics if area.y + offset_from_top is below the bottom of the screen
        let left = &mut surface[(area.left(), area.y + offset_from_top as u16)];

        left.set_style(selected);
        let right = &mut surface[(
            area.right().saturating_sub(1),
            area.y + offset_from_top as u16,
        )];
        right.set_style(selected);
    }
}

I've changed the logic for offset_from_top to:

let offset_from_top = (cursor - scroll).min(area.height as usize - 1).max(0);

Which seems to have fixed the problem.

Note: There could be a better way to fix this, but I don't know the codebase well enough to suggest one.

@5-pebbles
Copy link
Author

Actually when I compiled from the latest source it works fine, so something else must have fixed it since my last install.

¯\_( -_- )_/¯

@5-pebbles 5-pebbles closed this May 3, 2024
@pascalkuthe
Copy link
Member

Probably #10573

@5-pebbles
Copy link
Author

Yup that's it, thanks for fixing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@5-pebbles @pascalkuthe